Skip to content

fix: refresh display geometry for viewport screenshots - #3007

Merged
KazuCocoa merged 5 commits into
masterfrom
fix/duo-viewport-geometry
Oct 8, 2026
Merged

KazuCocoa merged 5 commits into
masterfrom
fix/duo-viewport-geometry

Conversation

@KazuCocoa

@KazuCocoa KazuCocoa commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Switching Duo displays leaves getScreenInfo, getDevicePixelRatio and getStatusBarHeight cached for the driver lifetime. The cached values can produce zero or negative viewport heights; cropping also uses app-window dimensions even when the selected display or captured image has different dimensions.

Remove these caches, read one fresh screen response per viewport calculation, and use WDA's display dimensions when available. Native viewport screenshots retain the captured image's full width and bottom edge and subtract only the current top status bar. Invalid crop heights now return the original screenshot as a best-effort result, with a diagnostic log message. Add seven regression tests and document the separate metadata/capture requests.

Relationship to existing Duo PRs

Targets master, independently of #2988 and #3001. #2988 owns display enumeration, settings and screenshot routing/fallback protection; #3001 owns hinge commands. They touch some of the same files but different functionality, and both PR diffs apply cleanly after this change.

Related to appium/appium#22857. The minimum WDA dependency is now ^16.14.1, including the companion screen-geometry fix. Removing driver caches alone cannot fix WDA's wrong-display metadata or hidden status-bar containers.

Validation

  • On the current master base: TypeScript build, lint, changed-file formatting and all 555 unit tests passed.
  • Earlier combined driver/WDA validation on Duo / iOS 27.1: closed → book → open → closed passed. Viewport images are 1398×2034 outside and 2852×2006 inside; the latter preserves WDA's existing one-pixel difference from nominal bounds.
  • iPhone 17 / iOS 26.5: a 54-point status bar is cropped by 162 pixels, producing a 1206×2460 viewport image from 1206×2622.
  • Unit tests cover changing scales, status-bar visibility, returning to a previous display, retry after a failed screen request, smaller app windows, image rounding/bounds, invalid crops and web-context delegation.

The live simulator validation used the companion WDA build with #1288 included. Physical Duo hardware and folding during capture remain unverified; tests should wait for stable layout and select the display explicitly.

Review follow-up: invalid crop geometry (whole-image removal, oversized or negative heights, and non-finite values) returns the captured image unchanged. This update was validated with unit tests; no new device run was performed.

Physical-device follow-up

With the integrated driver and WDA #1288 commit c0765edd, screenshot/viewport checks passed through portrait → landscape → portrait on iPad mini (A17 Pro), iPadOS 27.0.1, and iPhone SE (2nd generation), iOS 27.2. Portrait iPad images crop 1488×2266 to 1488×2202; SE images crop 750×1334 to 750×1294. Landscape iPad crops 2266×1488 to 2266×1424, while SE correctly retains the full 1334×750 image with no status bar.

The real iPad exposed an additional WDA portrait-orientation bug in the existing screen metadata implementation; #1288 fixes it. WDA 16.14.1 alone does not include that follow-up. The driver unit suite cannot substitute for the corrected WDA metadata. Full physical audit.

@KazuCocoa
KazuCocoa marked this pull request as ready for review October 7, 2026 04:11
Comment thread lib/commands/screenshots.ts Outdated
// including fractional-scale rounding in the captured image.
const top = Math.trunc(statusBarSize.height * scale);
if (!Number.isFinite(top) || top < 0 || top >= height) {
throw new errors.UnableToCaptureScreen(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it really be a breaking error? I assume we should just do a best-effort screenshot

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I kept the log for info, but it will return the screenshot result 98f9b66

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Negative fractional status-bar heights can bypass invalid-crop fallback and crop incorrectly.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates viewport screenshot geometry for dynamic displays by refreshing WDA metadata and using captured image dimensions.

Changes:

  • Removes cached screen geometry.
  • Improves display-aware viewport cropping and fallback handling.
  • Adds regression tests, documentation, and a WDA dependency update.
File Description
test/​unit/​commands/​screenshots.spec.ts Tests screenshot cropping and invalid geometry handling.
test/​unit/​commands/​general.spec.ts Tests refreshed geometry behavior.
package.json Updates the WDA dependency.
lib/​driver.ts Removes geometry memoization.
lib/​commands/​types.ts Adds optional display metadata fields.
lib/​commands/​screenshots.ts Crops screenshots using current geometry.
lib/​commands/​general.ts Refreshes screen metadata and calculates viewport rectangles.
docs/​reference/​execute-methods.md Documents fresh metadata and capture behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/commands/screenshots.ts Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Dynamic display geometry, screenshot cropping, and WDA behavior warrant final human validation.

0 open findings

1 resolved since last review

🧠 Review effort: Lite

@KazuCocoa
KazuCocoa merged commit 68d3103 into master Oct 8, 2026
24 of 25 checks passed
@KazuCocoa
KazuCocoa deleted the fix/duo-viewport-geometry branch October 8, 2026 03:42
github-actions Bot pushed a commit that referenced this pull request Oct 8, 2026
## [12.16.0](v12.15.2...v12.16.0) (2026-10-08)

### Features

* add simulated Duo hinge angle command ([#3001](#3001)) ([3f4d513](3f4d513))

### Bug Fixes

* refresh display geometry for viewport screenshots ([#3007](#3007)) ([68d3103](68d3103))
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 12.16.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions Bot pushed a commit that referenced this pull request Oct 21, 2026
## [13.0.0-beta.2](v13.0.0-beta.1...v13.0.0-beta.2) (2026-10-21)

### ⚠ BREAKING CHANGES

* Removed the asyncWaitMs driver property. Use scriptTimeoutMs (base driver) instead.
* GET /timeouts reports script: null for a new session, not 0.
* execute now uses the script timeout that the client sets (before: ignored). A client that sends 0, or a value shorter than its scripts, now gets script timeout.
* Removed the undocumented XCUITestDriver.executeAtomAsync method. It had no internal callers and was not exposed as a command. Driver subclasses that call it should use this.remote.executeAtomAsync(atom, args, this.curWebFrames) followed by this.waitForAtom(promise) instead.
* Removed the launchApp command and the POST /appium/app/launch route. Use mobile: launchApp instead.
* Removed the closeApp command and the POST /appium/app/close route. Use mobile: terminateApp instead.
* Removed the asyncScriptTimeout command and the POST /timeouts/async_script route. Use setScriptTimeout instead.
* Removed the no-op getSafariIsIphone driver method.
* Removed the no-op getSafariDeviceSize driver method.
* Removed the no-op getSafariIsNotched driver method.
* Removed the no-op getExtraTranslateWebCoordsOffset driver method.
* Removed the no-op getExtraNativeWebTapOffset driver method.
* Removed the deprecated hasElementId driver property.
* Removed the nativeWebTapTabBarVisibility setting. It had no effect, and setting it is no longer accepted.
* Removed the nativeWebTapSmartAppBannerVisibility setting. It had no effect, and setting it is no longer accepted.
* Removed the safariTabBarPosition setting. It had no effect, and setting it is no longer accepted.

### Features

* add asAppUnderTest flag in mobile: launchApp command ([#3004](#3004)) ([#3043](#3043)) ([d8163af](d8163af))
* add execute method for retrieving pressable buttons ([#3069](#3069)) ([bd109fc](bd109fc))
* add multi-display discovery and safe screenshot routing for Duo support ([#3013](#3013)) ([#3025](#3025)) ([cc15789](cc15789))
* add simulated Duo hinge angle command ([#3001](#3001)) ([fdf6128](fdf6128))
* add timeoutMs argument to mobile: stopPerfRecord ([#3003](#3003)) ([5071933](5071933))
* capture selected simulator displays through CoreSim fallback on beta ([#3060](#3060)) ([e8ba465](e8ba465))
* Remove deprecated APIs ([#3054](#3054)) ([b781cc3](b781cc3))
* support launching WDA on Duo ([#3002](#3002)) ([175ac5d](175ac5d))

### Bug Fixes

* backport native web tap calibration fixes to beta ([#3057](#3057)) ([492b3dd](492b3dd))
* backport picker wheel value and maxAttempts forwarding to beta ([#3023](#3023)) ([ec44a57](ec44a57)), closes [#3017](#3017)
* broadcast syslog to every logs websocket client ([#3005](#3005)) ([46ff47b](46ff47b))
* capture the whole element in web element screenshots ([#3039](#3039)) ([c2bda91](c2bda91))
* Drop obsolete sessions from the server list on IPC session claim ([#3024](#3024)) ([7c4c075](7c4c075))
* grepFile error handling ([#2989](#2989)) ([753d737](753d737))
* handle screen recorder shutdown failures on beta ([#3059](#3059)) ([4d12119](4d12119))
* honor explicit MJPEG screenshot orientation correction ([#3008](#3008)) ([4dbd5fd](4dbd5fd))
* include native geometry in web calibration cache ([#3009](#3009)) ([c3d0b47](c3d0b47))
* make tunnel-creation exit on Ctrl+C ([#3035](#3035)) ([349721a](349721a))
* parse unquoted and RFC 5987 Content-Disposition filenames ([#3026](#3026)) ([7f7ec27](7f7ec27))
* preserve app iframe selection after native tap calibration ([#3015](#3015)) ([bf1d804](bf1d804))
* preserve iOS 27 webview interaction history ([#3064](#3064)) ([57d0dbd](57d0dbd))
* preserve recording aspect ratio and input frame rate (beta backport) ([#3029](#3029)) ([7e935fd](7e935fd)), closes [#3010](#3010)
* preserve recording padding when MJPEG frame geometry changes ([#3061](#3061)) ([cb4e412](cb4e412))
* refresh display geometry for viewport screenshots ([#3007](#3007)) ([76fd082](76fd082))
* resolve WDA downloads from the monorepo release ([#3050](#3050)) ([#3051](#3051)) ([05e00f1](05e00f1))
* restore capability constraints removed from base driver ([#3028](#3028)) ([3fdf731](3fdf731))
* restore compatibility with beta capability and timeout types ([#3062](#3062)) ([39aa02a](39aa02a))
* stop dropping new crash reports in IOSCrashLog.getLogs ([#3031](#3031)) ([fa2d1f8](fa2d1f8))
* stop the running screen recording when forceRestart is set ([#3019](#3019)) ([5b57556](5b57556))
* track Apple TV tunnels by pair record UDID ([#3034](#3034)) ([5a00db7](5a00db7))
* use default sinon import for @types/sinon 22.0.1 compatibility ([#3068](#3068)) ([47eee58](47eee58))
* wait for a promise that a script returns in a Safari web context ([#3041](#3041)) ([f7b5524](f7b5524))

### Miscellaneous Chores

* bump Appium monorepo packages to latest betas ([#3020](#3020)) ([ac52334](ac52334))
* fix oxlint warnings ([#3055](#3055)) ([d2943ad](d2943ad))
* port some missing PRs to beta ([#3070](#3070)) ([a8c93f0](a8c93f0))

### Code Refactoring

* attach commands via prototype to allow overrides in descendants ([#3047](#3047)) ([533fcd5](533fcd5))
* remove unused driver.executeAtomAsync ([#3065](#3065)) ([f1729e6](f1729e6))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants